-
-
Notifications
You must be signed in to change notification settings - Fork 224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
!!! TASK: Interdimensional relatives for node move #4993
Conversation
... without prior distribution of siblings among different parents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...ContentGraph.DoctrineDbalAdapter/Tests/Behavior/Features/Projection/SiblingPositions.feature
Show resolved
Hide resolved
Neos.ContentRepository.Core/Classes/Feature/NodeMove/Event/NodeAggregateWasMoved.php
Show resolved
Hide resolved
Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Nodes.php
Outdated
Show resolved
Hide resolved
It's untouched; I guess it fails on a newer / older PHPStan version |
...and was fixed by just merging 9.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @nezaniel,
I went through your code, and I believe I understand the gist of it 😅 Left some comments, none of which are big things.
I also did some manual testing with the UI and ran the UI E2E tests atop your branch, and everything looks 👍
Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/NodeMove.php
Outdated
Show resolved
Hide resolved
Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/NodeMove.php
Outdated
Show resolved
Hide resolved
Neos.Neos/Classes/PendingChangesProjection/ChangeProjection.php
Outdated
Show resolved
Hide resolved
Neos.Neos/Classes/PendingChangesProjection/ChangeProjection.php
Outdated
Show resolved
Hide resolved
Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Nodes.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Wilhelm Behncke <2522299+grebaldi@users.noreply.github.com>
Co-authored-by: Wilhelm Behncke <2522299+grebaldi@users.noreply.github.com>
…ature/NodeMove.php Co-authored-by: Wilhelm Behncke <2522299+grebaldi@users.noreply.github.com>
…des.php Co-authored-by: Wilhelm Behncke <2522299+grebaldi@users.noreply.github.com>
Builds on: #4982
This replaces the cumbersome MoveNodeMappings DTO with the far more lightweight, but surprisingly even more capable InterdimensionalSiblings DTO.
Upgrade instructions
This is breaky for 3rd party packages since the event DTOs are part of the API
Review instructions
First: Don't be afraid of the number of LoC added. That's just proper test coverage for MoveNode, finally.
For the refactoring, further assumptions are made (and enforced via constraint checks and tests):
Pure edge operation
MoveNode is considered an pure edge operation, meaning no node is actually touched. This was also implied before, since MoveNode explicitly never updated any timestamps. Thus, no OrginDimensionSpacePoints are communicated anymore, just siblings per affected DSP.
Hint: Neos' ChangeProjection needs to decide how to deal with MoveNode. Its tests pass anyway.
Only one parent (at most) per move operation
As the RelationDistributionStrategy already states: the variants will be gathered (more or less, depending on the strategy) at the new parent, if given.
That also means: no implicit parent changes. If a given sibling belongs to a different parent, this will lead to
Succeeding sibling has priority over preceding sibling
The given succeeding sibling (and its further succeeding siblings) will be checked for applicability.
If none is found and a preceding sibling is given, it (and its further preceding siblings) will be checked instead.
Preceding siblings are evaluated if given and no succeeding sibling is given
Non-determinable siblings
If no succeeding sibling can be determined for the move operation, there are three cases
Checklist
FEATURE|TASK|BUGFIX
!!!
and have upgrade-instructions